[misc] fix fp8 #9742
[misc] fix fp8 #9742
Conversation
Summary of ChangesHello @hiyouga, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a minor but important bug in the parameter parsing logic. It rectifies an erroneous check that incorrectly validated FP8 training compatibility by looking at the wrong argument source. The fix ensures that the system correctly identifies when FP8 training is incompatible with quantization settings, improving the robustness of argument validation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue with argument validation for FP8 training by changing training_args.quantization_bit to model_args.quantization_bit. The change is correct and necessary. I've also identified a related bug in the same file concerning fp8 argument handling and have left a comment on the change with a suggestion to fix it as part of this PR for completeness.
| raise ValueError("GaLore and APOLLO are incompatible with DeepSpeed yet.") | ||
|
|
||
| if training_args.fp8 and training_args.quantization_bit is not None: | ||
| if training_args.fp8 and model_args.quantization_bit is not None: |
There was a problem hiding this comment.
This fix is correct! While reviewing, I noticed another related issue with fp8 argument handling in this file. On line 364, model_args.fp8 is assigned a value. However, the fp8 attribute is defined in TrainingArguments, not ModelArguments. This will cause an AttributeError if that code path is executed. To make this fp8 fix complete, could you please also change line 364 to training_args.fp8 = True?
What does this PR do?
Fixes # (issue)
Before submitting